Skip to content

feat(help): support JSON output for --help flags - #1337

Merged
BYK merged 4 commits into
mainfrom
issue-1265-json-help
Aug 2, 2026
Merged

feat(help): support JSON output for --help flags#1337
BYK merged 4 commits into
mainfrom
issue-1265-json-help

Conversation

@jared-outpost

@jared-outpost jared-outpost Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

sentry help --json returned structured help, but the --help forms agents reach for first (sentry --help --json, sentry issue --help --json, sentry issue list --help --json) fell through to Stricli's built-in text usage and ignored --json.

This rewrites a flag-based --help --json request to the dedicated help command during argv preprocessing (preprocessArgv), so all --help forms now emit the same JSON as sentry help --json. The rewrite only fires when both --help and --json are present before any -- escape — a bare --help keeps Stricli's text usage unchanged — and --fields is carried through for field selection.

Testing

  • pnpm exec vitest run test/lib/argv-hoist.test.ts test/lib/argv-hoist.property.test.ts test/commands/help.test.ts (89 passed)
  • tsc --noEmit clean, biome check clean on changed files
  • Manual: verified sentry --help --json, sentry issue --help --json, and sentry issue list --help --json produce valid JSON identical to the help command; bare --help still shows text usage; invalid command returns a JSON error (exit 60); -- tool --help --json after an escape is not rewritten

Closes #1265

Rewrite flag-based `--help --json` requests to the dedicated `help`
command in argv preprocessing, so `sentry --help --json` and
`sentry <command> --help --json` emit the same structured JSON as
`sentry help --json`. Agents and tooling reach for `--help` first, but
Stricli handles it internally and ignores `--json`.

Only fires when both `--help` and `--json` are present before any `--`
escape; a bare `--help` keeps Stricli's text usage output unchanged.
`--fields` is carried through for field selection.

Fixes #1265
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-02 13:46 UTC

@jared-outpost

jared-outpost Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

fix-ci: attempt 1 — the failure is library.test.ts bailing with "Bundle not built" in its beforeAll, while bundle.test.ts (which builds and exercises the same dist/index.cjs) passed in the same run. That points to a transient bundle-build race/timeout in the e2e harness, not the diff — this change only touches argv preprocessing (argv-hoist.ts) and its unit test, nothing in the esbuild bundle path. Re-running the failed jobs.

@jared-outpost
jared-outpost Bot marked this pull request as ready for review August 2, 2026 13:13
@github-actions github-actions Bot added the risk: high PR risk score: high label Aug 2, 2026
@jared-outpost jared-outpost Bot added the enhancement New feature or request label Aug 2, 2026
@jared-outpost

jared-outpost Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Marked ready for review. All CI is green — Unit Tests, E2E Tests, Lint & Typecheck, and the security/CodeQL/semgrep checks all passed (the earlier E2E failure was a flaky bundle-build race that cleared on re-run). Self-review found nothing to change.

Flagging that the risk-scoring workflow labeled this risk: high — since it touches argv preprocessing on every invocation, I'd like a human to sign off rather than auto-merging. Happy to address any review feedback.

Comment thread packages/cli/src/lib/argv-hoist.ts
Comment thread packages/cli/src/lib/argv-hoist.ts Outdated
Two issues in scanHelpJsonToken when a --help --json request carried
other flags:

- A dropped value flag kept its spaced value, so `--org acme` /
  `--limit 5` pushed `acme` / `5` into the command path and resolved the
  wrong command (or a not-found error). Value flags now drop their
  spaced value too; known boolean flags (`--verbose`) still leave the
  following token as a real path segment.
- `--fields` unconditionally consumed the next token, so `--fields
  --json` swallowed `--json` and the rewrite never fired. It now only
  takes a spaced value when the next token isn't a flag.

Reuses the existing GLOBAL_FLAGS metadata to tell boolean from
value-taking flags. Added tests for value-flag dropping, boolean-flag
path retention, and the --fields swallow case.
Comment thread packages/cli/src/lib/argv-hoist.ts
Comment thread packages/cli/src/lib/argv-hoist.ts Outdated
scanHelpJsonToken treated any --flag=value token as possibly
value-taking and dropped the following token as its spaced value. Since
isBooleanFlagToken looks up the full name including =value, an =-form
flag never matched a known boolean and its next token was discarded, so
`--org=acme issue list --help --json` resolved the wrong help path.

An =-form flag carries its value inline and never consumes a following
token, so skip the drop-next logic when the token contains =. Added a
test covering --org=acme / --limit=5 before the command path.
Comment thread packages/cli/src/lib/argv-hoist.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0d4f5d6. Configure here.

}
if (!token.startsWith("-")) {
scan.commandPath.push(token);
return 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Positionals leak into help path

Medium Severity

rewriteHelpJsonRequest treats every non-flag token as a command path segment, so positionals like an issue id are forwarded into help --json …. introspectCommand then rejects the extra segment and returns a not-found JSON error instead of help for the leaf command. Stricli’s bare --help ignores those positionals, so adding --json regresses those invocations from usable text help to an error.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0d4f5d6. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this as-is by design. The rewrite is scoped to command paths (sentry [<group> <command>] --help --json) — the forms agents actually use to discover a command's shape. When a positional like an issue id is present, forwarding it produces a structured JSON Command not found error (exit 60), which is the documented, machine-readable behavior for the JSON/agent use case this feature targets; it doesn't crash or hang. Making <command> <positional> --help --json fall back to the leaf command's help would require running route-tree resolution inside the argv preprocessor (argv-hoist.ts is currently a dependency-light string transform on the hot preprocessArgv path). Given this PR is already risk: high, I'd rather not couple the preprocessor to the command graph here — happy to do it as a follow-up if maintainers want the fallback.

for (let i = 0; i < argv.length; ) {
// Tokens after -- are positional/pass-through — a --help there is not ours.
if (argv[i] === "--") {
return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Escape aborts valid help rewrite

Low Severity

On encountering --, rewriteHelpJsonRequest returns null immediately even when --help and --json were already seen earlier in argv. The documented rule is only that those flags must appear before the escape; a later -- should stop scanning, not discard a rewrite that already qualified.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0d4f5d6. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional. Returning null on any pre--- scan hitting -- keeps the rule simple and conservative: a -- escape means everything after it is opaque pass-through, and I'd rather defer to Stricli's normal handling than commit to a JSON rewrite when an escape is in play (documented in the PR as -- tool --help --json not being rewritten). In practice --help --json ... -- combined with a trailing escape is not a form agents use for help discovery, so the extra complexity to salvage it isn't worth the risk on this path. Leaving as-is.

Stricli treats `-h` as a built-in alias for `--help`, but
scanHelpJsonToken only matched the literal `--help` token, so
`sentry -h --json` fell through to Stricli's text usage instead of
producing JSON. Recognize `-h` alongside `--help` and add tests.
@BYK

BYK commented Aug 2, 2026

Copy link
Copy Markdown
Member

Jared, would this be easier if we made a patch to Stricli itself (like we did for the -H argument) for everything in packages/cli/src/lib/argv-hoist.ts ?

@jared-outpost

jared-outpost Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

The `-H` patch works because it just deletes an alias registration from Stricli's scanner — a self-contained edit with no dependency on our code. `--help --json` is a different shape: the structured output comes from our own `introspectAllCommands`/`introspectCommand` in `src/lib/help.ts`, which a vendored @stricli/core dist patch can't reach. To do it in the patch we'd have to either reimplement JSON introspection inside the minified bundle or wire a callback back into our code — both worse than the current preprocessor.

There's also a maintenance angle: the patch is against dist/index.{cjs,js} and check-patches.ts already flags that every Stricli bump means re-running pnpm patch and reapplying the edits by hand. Adding help-JSON logic there grows that burden on every upgrade.

The preprocessor keeps it in normal TS next to the existing --version normalization, reuses the help command as the single JSON source (so sentry --help --json and sentry help --json can't drift), and is covered by unit + property tests. That's why I kept it out of the patch.

That said — if you'd rather push the --help/--json interception upstream into the patch for consistency with -H, I'm happy to prototype it; just flagging the tradeoffs first.

@BYK

BYK commented Aug 2, 2026

Copy link
Copy Markdown
Member

That said — if you'd rather push the --help/--json interception upstream into the patch for consistency with -H, I'm happy to prototype it; just flagging the tradeoffs first.

I'd rather have this upstream but in a simpler way: just support "top-level args" concept so we don't need any pre-processing. This applies to everything in that preprocessor file like --verbose or --log-level

@jared-outpost

jared-outpost Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Agreed, that's the better shape — a "top-level flags" allow-list in the scanner (buildRouteScanner already special-cases --help/--helpAll/--version as it walks routes, so it's the natural hook) would let --verbose, --log-level, --json, --fields, --org, --project be recognized at any depth and drop the whole argv-hoist.ts preprocessor, not just the help-json rewrite.

One nuance to sequence around: --verbose/--log-level/--org/--project are leaf flags buildCommand injects, so once the scanner stops treating them as route segments the leaf parser just handles them — clean. --help --json is different: --help is terminal in the scanner (prints text usage before any command runs) and the JSON comes from our introspectCommand/introspectAllCommands. So even with top-level flags, something still has to decide "help + json → structured output" and call back into our introspection. That's a small piece of app glue on top of the scanner change, not free.

How do you want to sequence it? Options I see:

  1. Land this PR as the interim fix (unblocks agents today), and I open a follow-up issue for the top-level-flags patch that subsumes the whole preprocessor.
  2. Convert this PR into the top-level-flags approach directly and drop the argv-hoist layer here.

I lean toward (1) since the refactor touches every global flag and wants its own review, but happy to do (2) if you'd rather not ship the preprocessor at all. Your call.

@BYK

BYK commented Aug 2, 2026

Copy link
Copy Markdown
Member

Go with 1. That said I'd argue we should allow modifying --help behavior even with others like --verbose so "terminal" flags still should parse others and pass them to their handlers. They can be ignored but for the cases we care about, we will handle (such as the --json case)

Feel free to submit PRs to upstream both for this and -H when you are done and we'll see if they accept it or not.

@jared-outpost

jared-outpost Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Sounds good. Filed #1339 for the top-level-flags refactor and captured your terminal-flag note there — --help stays terminal for routing but still parses siblings like --json and forwards them to the handler, so structured help falls out naturally instead of being a special-case rewrite. The upstream Stricli PRs (top-level flags + -H) are on that issue's task list; I'll open them once the local patch implementation is in place.

This PR is unchanged and ready as the interim fix — leaving the merge call to you given the risk: high label.

@BYK
BYK merged commit bf2d77d into main Aug 2, 2026
32 checks passed
@BYK
BYK deleted the issue-1265-json-help branch August 2, 2026 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request risk: high PR risk score: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support JSON output for --help flags

1 participant